E2E: replace login priming with an on-demand lock - #113352
Draft
lucatume wants to merge 5 commits into
Draft
Conversation
|
Looks like one of the E2E tests has failed. You can fix them following these steps:
|
lucatume
force-pushed
the
update/e2e-login-lock
branch
from
August 6, 2026 17:20
c59c927 to
f428704
Compare
Contributor
|
WordPress.com
Automattic for Agencies
|
Workers share one cookies file per account through COOKIES_PATH, and a CI build starts with none of them. Every worker misses at once and logs in concurrently, against a calypso.live container that has just been created. TestAccount.logInOncePerRun coordinates through a lock directory beside the cookies: the worker that creates it logs in and writes the cookies, the rest poll for those cookies and skip the login. A lock older than 90s belongs to a worker Playwright tore down mid-login and gets taken over by rename, so only one of several waiters can claim it. A waiter that has waited a minute logs in unlocked rather than burn its test timeout. The lock is advisory: nothing stops a worker whose login outran the stale window from writing cookies anyway. That costs the concurrent login the lock exists to avoid, which is where every login was before it, and saveAuthCookies renames its file into place, so a reader never reads a torn one. authenticate() now saves the cookies it obtains, which it never did: the accounts reached only through it re-logged-in on every test. It is also the only place that logs in now, so an account fixture is authenticated when it is handed over rather than in each spec that remembers to ask.
The setup project logged in as a per-build-type list of accounts before the suite, so the specs would find cookies rather than all log in at once. The list is hand written per test group: it drifts from what the specs use, an account missing from it got no priming at all, and a build type running the wrong list paid for logins no spec needs. The login lock covers the same ground without a list. Removes the project, the env var behind it, the resolver and the reporting script that read it, the 25 TeamCity parameters setting it and the semicolon form EXTRA_ENV_VARS grew to carry one. Also the priming half of the legacy Jest global setup, which has had no specs to prime since the last one moved to Playwright Test.
Calypso home hands a logged-in user over to the Dashboard on another host, and that navigation starts after the load event. A spec that authenticates with waitUntilStable false and navigates straight away raced it and lost: reader__view failed both its attempts on both devices with ERR_ABORTED on the goto to /reader, with the Dashboard shell on screen. The prime-logins project hid this. It logged in as the account before the suite, so by the time the spec ran the container had served the same pages already and the hand-over fired earlier than the spec's own navigation. Such a caller navigates itself next, so authenticate() no longer loads Calypso home for it, and it waits for the URL to stop changing before it returns.
lucatume
force-pushed
the
update/e2e-login-lock
branch
from
August 10, 2026 10:43
f428704 to
75acde2
Compare
The account fixture ran a full authenticate(), sidebar wait included, so the specs that pass waitUntilStable: false paid the wait they opt out of and the rest made the Calypso home trip twice. It stops at the cookies now. The lock holder removed the lock by path, so a login slower than LOCK_STALE_MS deleted the lock of the worker that had taken it over and let the others log in at once. The lock carries its owner's token and only they remove it. Logging in happens in fixture setup and was charged to the test's own 120s timeout, so a worker that waited out LOCK_WAIT_MS had under a minute left for the login and the test both. getAccount hands that time back. Smaller ones: the first login attempt's error was swallowed and its retry immediate, the login never dismissed the cookie banner that covers the submit button on mobile viewports, and the settle wait took a single quiet poll for done and said nothing when it timed out.
AUTHENTICATE_ACCOUNTS is gone, but saved custom runs and personal builds still carry the semicolon form. Read as a single pair it fails the first env cast and takes the build down with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
TestAccount.logInOncePerRun()logs an account in behind a lock directory inCOOKIES_PATH, so a run logs each account in once rather than once per worker.prime-loginssetup project,AUTHENTICATE_ACCOUNTS, its resolver and reporting script, and the 25 TeamCity params that set it. This supersedes E2E: prime login cookies once before the suite #113259 and E2E: prime only the accounts each build's test group logs in as #113339, which are being deleted here.authenticate()no longer returns with a navigation in flight.Why are these changes being made?
The priming list is written by hand per test group. It drifts from what the specs use, an account missing from it gets no priming at all, and a build type carrying the wrong list pays for logins no spec needs. The lock covers the same ground without a list.
Testing Instructions
Three personal builds of
E2E Tests (Playwright Test), all on current trunk:reader__viewfailed both attempts on both devices,ERR_ABORTEDon the goto to/reader. The trace shows Calypso home handing the user to the Dashboard on another host mid-navigation.prime-loginswas hiding it.reader__viewgreen on both devices. The only failures left areplugins__search,statsandinvite__new-user, all red on trunk since build Jetpack Connect: Introduce NUX Site User Type step. #31297.Run 18877819 carried
DEBUG=test-account, which prints one line per login. Nine accounts, nine logins, 40 workers reading the cookies instead:defaultUser1/12,gutenbergSimpleSiteUser1/10,simpleSiteFreePlanUser1/7,simpleSitePersonalPlanUser1/5,atomicUser1/3,calypsoPreReleaseUser1/3, three more at 1/0. No worker hit the wait timeout or found an abandoned lock.Locally,
yarn jest --config packages/calypso-e2e/jest.config.js --rootDir packages/calypso-e2ecovers the lock: 8 concurrent workers producing one login, release on success and on throw, stale-lock takeover, retry, and the wait ending on freshness.What a reviewer should check. The waiter's exit condition is
hasFreshAuthCookies(), not the cookies file existing: a file left by an earlier run can be there and expired, and waiting on existence hands every waiter a dead session. And the lock is deliberately advisory — a worker whose login outruns the stale window can write cookies while another holds the lock. That costs one duplicate login, which is where every login was before this PR;saveAuthCookiesrenames its file into place, so no reader sees a torn one.Pre-merge Checklist